Java: Replace SSA wrapper classes with shared implementation. by aschackmull · Pull Request #20761 · github/codeql

aschackmull · 2025-11-05T10:35:30Z

This migrates the Java SSA libraries to use the shared SSA wrapper classes.

Commit-by-commit review is very much encouraged.

java/ql/lib/semmle/code/java/dataflow/SSA.qll


 import java
 private import internal.SsaImpl
+import internal.SsaImpl::Ssa as Ssa


java/ql/lib/semmle/code/java/dataflow/TypeFlow.qll

  }

-  private predicate upcastEnhancedForStmtAux(BaseSsaUpdate v, RefType t, RefType t1, RefType t2) {
+  private predicate upcastEnhancedForStmtAux(


java/ql/lib/semmle/code/java/dataflow/internal/BaseSSA.qll

-      i = 0
-    )
-  }
+module Ssa = Impl::MakeSsa<SsaInput>;


java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll

+  }
+}
+
+module Ssa = Impl::MakeSsa<SsaInput>;


aschackmull · 2025-11-07T11:51:14Z

I've left several deprecation annotations for a followup PR to avoid having to do a submodule bump here.

Copilot

Pull Request Overview

This PR modernizes the SSA (Static Single Assignment) API for Java CodeQL by renaming classes and methods to improve clarity and consistency. The changes deprecate old class and method names while introducing clearer alternatives.

Renames core SSA classes (e.g., SsaVariable → SsaDefinition, SsaPhiNode → SsaPhiDefinition)
Renames methods for consistency (e.g., getAUse() → getARead(), getCfgNode() → getControlFlowNode())
Introduces new specialized SSA classes with clearer semantics (SsaCapturedDefinition, SsaImplicitCallDefinition, etc.)

Reviewed Changes

Copilot reviewed 53 out of 53 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
shared/ssa/codeql/ssa/Ssa.qll	Adds toString methods and renames SsaPhiDefinition class definition
java/ql/lib/semmle/code/java/dataflow/SSA.qll	Core SSA API with renamed classes and deprecated old names
java/ql/lib/semmle/code/java/dataflow/internal/BaseSSA.qll	Base SSA implementation updated with new class names
java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll	Internal SSA implementation restructuring
java/ql/test/*/.ql	Test queries updated to use new API
java/ql/test/*/.expected	Generated test expectations updated with new toString formats
java/ql/lib/*/.qll	Various library files updated to use new API
java/ql/src/*/.qll	Source queries updated to use new API
java/ql/lib/semmle/code/java/Expr.qll	Adds new VariableWrite class for SSA integration
java/ql/lib/change-notes/2025-10-07-ssa-api-updates.md	Documents the deprecation of old SSA API

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…teDefinition.

…ger tracked as SSA variables.

hvitved

LGTM; my main concern is about caching.

java/ql/lib/semmle/code/java/dataflow/SSA.qll

hvitved · 2025-11-17T09:56:57Z

java/ql/lib/semmle/code/java/dataflow/internal/BaseSSA.qll

+private import Cached
+
+cached
+private module Cached {


Did you check that this cached stage is collapsed with the one inside Impl::MakeSsa?

Yes. See below.

hvitved · 2025-11-17T09:59:38Z

java/ql/lib/semmle/code/java/dataflow/internal/SsaImpl.qll

+  }
+}
+
+module Ssa = Impl::MakeSsa<SsaInput>;


Same question: Did you check that the cached stages are collapsed?

hvitved · 2025-11-17T10:24:25Z

java/ql/lib/semmle/code/java/dataflow/NullGuards.qll

+    result = f.getAnAccess() and
+    f.isFinal() and
+    f.getInitializer() = clearlyNotNullExpr(reason) and


Some of this logic is duplicated inside clearlyNotNull(SsaDefinition v, Expr reason), so perhaps factor out?

True, but we want to cover the case in both predicates, so we can't quite eliminate either of the cases. We could factor out another mutually recursive predicate clearlyNotNullField(Field f, Expr reason) and refer to that twice, but it's a bit borderline whether there's actually enough common code for that, so I'm leaning slightly towards keeping it as-is.

aschackmull · 2025-11-18T11:36:08Z

Regarding caching: I've compared the outputs of the stageoverlap script and the number of stages is unchanged. Two stages change as expected:
BaseSSA stage before:

STAGE 11:
18 cached predicate(s)
  BaseSSA::Impl::TPhiReadNode#aebc7af2
  `BaseSSA::Cached::captures/2#8a9d4eca`
  `BaseSSA::baseSsaAdjacentUseUseSameVar/2#705c4532`
  `BaseSSA::baseSsaAdjacentUseUse/2#c9616c05`
  `BaseSSA::Cached::ssaUpdate/2#862a1ebc`
  `BaseSSA::Cached::ssaDefReachesEndOfBlock/2#78b18ca2`
  `BaseSSA::Cached::phiHasInputFromBlock/3#c7e7d002`
  `BaseSSA::Cached::firstUse/2#bd1a5768`
  `BaseSSA::Cached::ssaImplicitInit/1#4eea6057`
  `BaseSSA::Cached::getAUse/1#757edc8c`
  `BaseSSA::BaseSsaSourceVariable.getAnAccess/0#dispred#63a9bea2`
  `BaseSSA::BaseSsaStage::backref/0#74f67f6e`
  `BaseSSA::BaseSsaStage::ref/0#98b08ed3`
  BaseSSA::TLocalVar#427c4922
  BaseSSA::TBaseSsaSourceVariable#97b3067c
  BaseSSA::Impl::TPhiNode#d7813977
  BaseSSA::Impl::TDefinitionExt#51480cc1
  BaseSSA::Impl::TWriteDef#87bef8d1

BaseSSA stage after:

19 cached predicate(s)
  BaseSSA::Impl::TPhiReadNode#cf0be3af
  `BaseSSA::Cached::captures/2#8a9d4eca`
  `BaseSSA::baseSsaAdjacentUseUseSameVar/2#705c4532`
  `BaseSSA::baseSsaAdjacentUseUse/2#c9616c05`
  `BaseSSA::Ssa::Cached::uncertainWriteDefinitionInputCached/2#814503bc`
  `BaseSSA::Ssa::ssaDefReachesUncertainRead/4#076b6de1`
  `BaseSSA::Ssa::Cached::phiHasInputFromBlockCached/3#29383672`
  `BaseSSA::Ssa::Cached::parameterInit/2#8fc0e46e`
  `BaseSSA::Ssa::Cached::isLiveAtEndOfBlock/2#f8ebed08`
  `BaseSSA::Ssa::Cached::explicitWrite/2#1b6c0012`
  `BaseSSA::Ssa::Cached::ssaDefReachesCertainRead/2#c11820eb`
  `BaseSSA::BaseSsaSourceVariable.getAnAccess/0#dispred#63a9bea2`
  `BaseSSA::BaseSsaStage::backref/0#74f67f6e`
  `BaseSSA::BaseSsaStage::ref/0#98b08ed3`
  BaseSSA::TLocalVar#427c4922
  BaseSSA::TBaseSsaSourceVariable#97b3067c
  BaseSSA::Impl::TPhiNode#1a14f911
  BaseSSA::Impl::TDefinitionExt#130e8dc7
  BaseSSA::Impl::TWriteDef#0bf8b8b7

SSA stage before:

STAGE 21:
27 cached predicate(s)
  `SsaImpl::firstUse/2#e5f95b1c`
  `SsaImpl::relevantFieldUpdate/3#175bdda3`
  `SsaImpl::defUpdatesNamedField/3#b177d49b`
  `SsaImpl::captures/2#ebc1b8ae`
  `SSA::adjacentUseUseSameVar/2#68cf25c3`
  `SSA::adjacentUseUse/2#c9086f2d`
  SsaImpl::DataFlowIntegrationImpl::TSsaInputNode#8f227fe5
  `SsaImpl::ssaUntrackedDef/2#51aeffed`
  `SsaImpl::ssaDefReachesUncertainDef/2#1e11fc24`
  `SsaImpl::ssaDefReachesEndOfBlock/2#ea5b1b02`
  `SsaImpl::phiHasInputFromBlock/3#89518051`
  `SsaImpl::ssaImplicitInit/1#e06df881`
  `SsaImpl::ssaExplicitUpdate/2#d4539208`
  `SsaImpl::ssaUncertainImplicitUpdate/1#11315a30`
  `SsaImpl::getAUse/1#ca5d2675`
  SsaImpl::Impl::TPhiReadNode#357c6989
  `SsaImpl::DataFlowIntegrationImpl::getAPhiInputDef/2#6c94e4b7`
  SsaImpl::DataFlowIntegrationImpl::TSsaDefinitionNode#4e3b9ed8
  SsaImpl::DataFlowIntegrationImpl::TNode#d672eac3
  SsaImpl::DataFlowIntegrationImpl::TExprNode#60cb727e
  SsaImpl::DataFlowIntegrationImpl::TWriteDefSource#5e63c235
  `SsaImpl::DataFlowIntegration::localMustFlowStep/3#b7be827d`
  `SsaImpl::DataFlowIntegration::localFlowStep/4#3d967e2f`
  `SsaImpl::getDestVar/1#25a31530`
  SsaImpl::Impl::TPhiNode#428e419d
  SsaImpl::Impl::TDefinitionExt#672b6ca5
  SsaImpl::Impl::TWriteDef#3b2d2270

SSA stage after:

STAGE 21:
26 cached predicate(s)
  `SsaImpl::firstUse/2#e5f95b1c`
  `SsaImpl::relevantFieldUpdate/3#175bdda3`
  `SsaImpl::defUpdatesNamedField/3#b177d49b`
  `SsaImpl::captures/2#ebc1b8ae`
  `SSA::adjacentUseUseSameVar/2#68cf25c3`
  `SSA::adjacentUseUse/2#c9086f2d`
  SsaImpl::DataFlowIntegrationImpl::TSsaInputNode#2de8f875
  `SSA::Ssa::Cached::uncertainWriteDefinitionInputCached/2#5657081b`
  `SSA::Ssa::ssaDefReachesUncertainRead/4#b539c176`
  `SSA::Ssa::Cached::phiHasInputFromBlockCached/3#19163e7b`
  `SSA::Ssa::Cached::parameterInit/2#6aae25ba`
  `SSA::Ssa::Cached::isLiveAtEndOfBlock/2#6431964a`
  `SSA::Ssa::Cached::explicitWrite/2#6c9f09cd`
  `SSA::Ssa::Cached::ssaDefReachesCertainRead/2#cda75555`
  SsaImpl::Impl::TPhiReadNode#9d9a39b2
  `SsaImpl::DataFlowIntegrationImpl::getAPhiInputDef/2#eb9fec7a`
  SsaImpl::DataFlowIntegrationImpl::TSsaDefinitionNode#c0ad6d68
  SsaImpl::DataFlowIntegrationImpl::TNode#c0ec5eed
  SsaImpl::DataFlowIntegrationImpl::TExprNode#00fd7dbc
  SsaImpl::DataFlowIntegrationImpl::TWriteDefSource#2c42c1d0
  `SsaImpl::DataFlowIntegration::localMustFlowStep/3#b7be827d`
  `SsaImpl::DataFlowIntegration::localFlowStep/4#3d967e2f`
  `SsaImpl::getDestVar/1#25a31530`
  SsaImpl::Impl::TPhiNode#4eb75855
  SsaImpl::Impl::TDefinitionExt#27006a88
  SsaImpl::Impl::TWriteDef#edb3bd92

hvitved

Thanks for verifying that caching works as expected.

github-actions bot added the Java label Nov 5, 2025

aschackmull force-pushed the java/ssa-shared branch from e54b277 to cf8d1bd Compare November 5, 2025 10:37

github-advanced-security bot found potential problems Nov 5, 2025

View reviewed changes

aschackmull force-pushed the java/ssa-shared branch 6 times, most recently from aad896a to cb33a68 Compare November 7, 2025 10:30

aschackmull mentioned this pull request Nov 7, 2025

Java: Add deprecation annotations in SSA #20772

Merged

github-actions bot added the documentation label Nov 7, 2025

aschackmull marked this pull request as ready for review November 7, 2025 11:51

aschackmull requested a review from a team as a code owner November 7, 2025 11:51

Copilot AI review requested due to automatic review settings November 7, 2025 11:51

aschackmull requested a review from a team as a code owner November 7, 2025 11:51

Copilot AI reviewed Nov 7, 2025

View reviewed changes

aschackmull added 13 commits November 12, 2025 09:06

Java: Move SSA entry defs to index -1.

c4f0868

Java: Get rid of untracked SSA definitions.

f2181ec

Java: Remove getAFirstUse in BaseSSA.

374c772

SSA: Fix phi defs.

79b2f21

SSA: Improve toString.

289d337

Java: Add VariableWrite class.

551944b

Java: Replace BaseSSA class wrappers with shared code.

942dc2b

Java: Instantiate shared SSA wrappers for main SSA.

d5708fd

Java: Simplify instantiation of Guards and ControlFlowReachability.

154f077

Java: Replace usages of isParameterDefinition.

99aa033

Java: Replace getAFirstUse with top-level predicate.

07e6356

Java: Replace uses of SsaExplicitUpdate.

483b2d8

Java: Introduce SsaCapturedDefinition and replace uses of getAnUltima…

06df5c0

…teDefinition.

aschackmull added 12 commits November 12, 2025 09:06

Java: Update some qldoc deprecation notices.

3e43c53

Java: Replace SsaPhiNode with SsaPhiDefinition.

35caede

Java: Replace getAUse with getARead.

f4b9efc

Java: Replace remaining SsaImplicitInit.

8594ae0

Java: Replace usages of SsaVariable.

f0bd034

Java: Update toString for implicit writes.

ee5d65e

Java: Deprecate two more SSA classes.

5849d85

Java: Drop caching of deprecated predicates.

95ac61d

Java: Accept toString changes in qltest.

e059ded

Java: Accept qltest changes due to dropped UntrackedDef.

109a5eb

Java: Add change note.

437ca58

Java: Reinstate useless null check results for fields that are no lon…

4a58a01

…ger tracked as SSA variables.

aschackmull force-pushed the java/ssa-shared branch from 3a30e2a to 4a58a01 Compare November 12, 2025 08:06

hvitved reviewed Nov 17, 2025

View reviewed changes

hvitved approved these changes Nov 18, 2025

View reviewed changes

aschackmull merged commit fe7be22 into github:main Nov 18, 2025
52 of 53 checks passed

aschackmull deleted the java/ssa-shared branch November 18, 2025 12:31

Conversation

aschackmull commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Check warning

Check warning

Uh oh!

Check warning

Check warning

aschackmull commented Nov 7, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

hvitved left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hvitved Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

aschackmull Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

hvitved Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

hvitved Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

aschackmull Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

aschackmull commented Nov 18, 2025

Uh oh!

hvitved left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aschackmull commented Nov 5, 2025 •

edited

Loading